-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add 19.03 release to changelog #1537
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1537 +/- ##
=======================================
Coverage 91.45% 91.45%
=======================================
Files 18 18
Lines 1791 1791
Branches 339 339
=======================================
Hits 1638 1638
Misses 130 130
Partials 23 23 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine, but I would make the issue numbers into links.
Thanks for the update 💪
I have a small request: make changelogs more detailed. A raw list of closed issues is a bad changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a much-needed PR. Thanks for taking care of this.
CHANGELOG.md
Outdated
19.03.1 | ||
- Fixes: | ||
- #1529 Update project PyPI credentials | ||
19.03.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this is not available on PyPi. So, we can merge both the change logs under 19.03.1
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. If we really want to have it there for consistency, I would say to put everything under 19.3.1
, and then have a note for 19.3.0
that says something like "skipped for package management purposes, not released on PyPI".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate that pypi
changes the version to number based and removes leading zeroes. 🙄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seemethere There's be an open issue on the setuptools
for forever on this pypa/setuptools#308.
Technically on the pypi side, 19.03.1 == 19.3.1
because of PEP-440
All release segments involved in the comparison MUST be converted to a consistent length by padding shorter segments with zeros as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but visually for calver 19.3.1
looks so much worse than 19.03.1
since it becomes inconsistent if you ever decide to release between October through December.
🤷♂️, I guess setuptools
is the ultimate authority on versioning.
Yes, very good point, I much agree. I'll go through these a bit later today |
So, in 63179ba I've started to go through each PR and expand each of items a bit, following @andreymal's point (#1537 (comment)). It's taking a bit of time. Does it look like I'm headed in the right direction? |
@cakemanny Yes you are in the right direction. I think some items may have been missed from your list though. ➜ git log 18.12.0..19.3 --oneline | grep '#'
abf8534 fix typo in Asyncio example (#1510)
773a66b Fix typo (#1516)
269100e format: fix linter issue causing travis build failures (fix #1514) (#1515)
2a15583 add Request.not_grouped_args, deprecation warning Request.raw_args (#1476)
d581315 Allow sanic test client to bind to a random port (#1376)
348964f Enable Middleware Support for Blueprint Groups (#1399)
e5c7589 Remove update_current_time refresh (#1502)
4260528 Fix the auto_reloader to work when the executable was launched with a module, rather than a script. (#1501)
34fe26e Add Route Resolution Benchmarking to Unit Test (#1499)
8a59907 Recognizes non-ASCII filenames in RFC 2231, and suport filename length is zero for multipart/form-data. (#1497)
52deeba Merge pull request #1490 from chenjr0719/fix_doc_build
ab56af5 Merge pull request #1489 from tomchristie/patch-1
42bf103 Remove deleted repo (#1487)
c8d2a46 did you mean specific? (#1486)
08794ae Enforce Datetime Type for Expires on Set-Cookie (#1484)
4f70dba sanic-zipkin (#1483)
b926a2c sanic#1480 Allow negative int/number in path (#1481)
52bdd1d Add stream support for bp.add_route() (#1482)
bc7d0f0 Merge pull request #1478 from chenjr0719/fix_doc_build
2758a3a Merge pull request #1472 from xxNB/dev
ef3c9ea Merge pull request #1477 from kyb3r/patch-2
9cf2e1b Merge pull request #1470 from denismakogon/create-server
99f34c9 Merge pull request #1457 from huge-success/max-age-integer
2af229e Merge pull request #1445 from huge-success/r0fls-977
8dd8e99 upgrade pytest version that compatible with pytest-cov, fixes some caplog unit tests (#1464)
96af1fe Merge pull request #1460 from huge-success/18.12-changelog
52de354 Merge pull request #1442 from Amanit/feature/gunicorn-logging
f4f90ca Merge pull request #1449 from chenjr0719/add_amending_request_object_example
cea1547 Merge pull request #1446 from huge-success/ahopkins-patch-1
fd5ae01 Merge pull request #1444 from ja8zyjits/master
19b4283 Merge pull request #1 from ja8zyjits/ja8zyjits-patch-1-readme
ff38a3c Merge pull request #1443 from huge-success/new-readme
72f2e18 Merge pull request #1440 from harshanarayana/fix/Contribution_Guide_Pip_Install
13804dc Merge pull request #1424 from harshanarayana/enh/Documentation_Update
67d51f7 Merge pull request #1423 from yunstanford/request-streaming-support |
I think this points at a need we should try and address: updates to the changelog during the PR, and not just after the release. |
Absolutely. Couldn't agree more. We can add a check in the PR to validate if the changelogs are updated as well. |
In this case I have another small note: «fixed typo» changes are actually just a flood which not everyone is interested, perhaps it would be nice to sort this by importance (security fixes / new features / major bug fixes / minor fixes) or not even mention typos in the changelog because they are too minor (but it is debatable) |
I don't think it's important to keep the exact format here (pyup seems to have trouble parsing I think I've noticed) but it would make sense to consider the ideas here: https://keepachangelog.com/en/1.0.0/#how. Some of them have already been touched on. From my point of view as a user of Sanic, breaking changes are what I care most about seeing in the changelog. |
@cakemanny Thanks for sharing the input. I think there is sort of a decision we need to make about change log and whether it should be the same as the release notes or not. I think I can agree that there needs to be an easily digestable place for users to figure out what new features there may be, and what deprecations may be coming (without all the minor changes). |
Still a work in progress, but in 638192b I've gone through the rest of the PRs that were in the list I started with. I've moved the ones I don't think are significant down to their own sections, and have expanded on each of the changes and fixes that will impact consumers of the library. |
- [#1470](https://github.com/huge-success/sanic/pull/1470) | ||
Added 2 new parameters to `sanic.app.Sanic.create_server`: | ||
- `return_asyncio_server` - whether to return an asyncio.Server. | ||
- `asyncio_server_kwargs` - kwargs to pass to `loop.create_server` for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not sure I understand this this PR.
As far as I can tell the default is now just broken.
https://github.com/huge-success/sanic/pull/1470/files#diff-c4444a35dbd5e37ee480b0e8888e0880R1196
This changes the default from run_async=True
to run_async=False
In which case... we need a known issues section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at the comment here: #1470 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That hasn't helped me I'm afraid. Sorry.
both of these programs run on 18.12 but error on 19.3:
import asyncio, sanic
loop = asyncio.new_event_loop()
app = sanic.Sanic()
server = loop.run_until_complete(app.create_server())
import sanic, uvloop
loop = uvloop.new_event_loop()
app = sanic.Sanic()
server = loop.run_until_complete(app.create_server())
I take it this is unintentional, and is a bug I should raise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work here adding some good context!
I think I'm done adding proper summaries for each of the changes and fixes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind squashing your commits? I think it'd be cleaner when looking back at the history. Otherwise lgtm
@cakemanny 👍 Great job. Thanks for taking care of this. |
I am creating a new thread of the community forum for the same so that we can continue the discussion there without breaking the chain here in this PR.
|
d16fa22
to
82f1cb6
Compare
Sure, no problem. Squashed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I've lifted the list of changes for 19.03 from the releases section on github.
The style is quite different from the 18.12 but conversely that varies a bit from the 0.8.0 section - is that a problem?